feat(ascend): add Device injection policy and fix A5 driver/UB mounts - #25
feat(ascend): add Device injection policy and fix A5 driver/UB mounts#25yxf0314 wants to merge 1 commit into
Conversation
On Docker, the default Env injection policy sets the visible-devices env (ASCEND_VISIBLE_DEVICES), which makes the Ascend Docker Runtime apply device isolation. On A5 (950) that isolation hides the UB fabric, so HCCL rootInfo detection fails. The CDI mount profile, modeled on the A2/A3 operator profile, also omits A5's UB driver components (ube_mgmt, under the driver tree) and blanket-mounts a host /etc/hccl_rootinfo.json whose stale content fails rootInfo detection. - Add a "Device" resource injection policy: inject device nodes and mounts as plain Docker devices/binds (reusing the CDI generator), with no visible-devices env and without requiring CDI-capable Docker. - Drop mirrored visible-devices envs under non-Env policies so a mirrored ASCEND_VISIBLE_DEVICES cannot re-trigger the isolation. - For A5, mount the whole /usr/local/Ascend/driver (brings ube_mgmt) and stop mounting /etc/hccl_rootinfo.json. - Keep injected mounts when appending container mounts, and fix a device mirroring typo exposed by the new plain-device path.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new 'Device' resource injection policy for the Docker deployer, allowing direct injection of device nodes and mounts to bypass the visible-devices environment variable, which resolves issues with the Ascend A5 UB fabric. It also refactors resource parsing and updates Ascend CDI configuration generation to mount the entire driver directory for A5 devices. The review feedback correctly identifies a critical bug in _inject_devices_plain where host and container paths are inverted when constructing the Docker device mapping string, and provides a code suggestion to fix it.
| for dn in device_nodes: | ||
| if dn.path in seen: | ||
| continue | ||
| seen.add(dn.path) | ||
| devices.append( | ||
| f"{dn.host_path or dn.path}:{dn.path}:{dn.permissions or 'rwm'}", | ||
| ) |
There was a problem hiding this comment.
In device_to_cdi_device_node (defined in gpustack_runtime/deployer/cdi/__utils__.py), the parameters are passed to ConfigDeviceNode as ConfigDeviceNode(path=dev.path, host_path=container_path). This means that for any dn (a ConfigDeviceNode):
dn.pathactually holds the host path.dn.host_pathactually holds the container path.
As a result, constructing the Docker device mapping string as f"{dn.host_path or dn.path}:{dn.path}" incorrectly maps container_path:host_path instead of host_path:container_path. This inversion will cause Docker to fail to mount the device correctly or fail to start the container.
Additionally, the duplicate check if dn.path in seen: compares the host path against container paths in seen (since seen extracts the container path from existing devices).
We should fix this by correctly resolving the container path and host path, and checking/adding the container path in seen.
| for dn in device_nodes: | |
| if dn.path in seen: | |
| continue | |
| seen.add(dn.path) | |
| devices.append( | |
| f"{dn.host_path or dn.path}:{dn.path}:{dn.permissions or 'rwm'}", | |
| ) | |
| for dn in device_nodes: | |
| container_path = dn.host_path or dn.path | |
| if container_path in seen: | |
| continue | |
| seen.add(container_path) | |
| devices.append( | |
| f"{dn.path}:{container_path}:{dn.permissions or 'rwm'}", | |
| ) |
|
Verified on a real 8× Ascend950PR host (card-4p, driver 25.7.rc1.6, ascend-docker-runtime v26.1.0, How this PR actually fixes it, and why it needs both halvesThe failure chain I measured:
Ranktable format is bound to the chip generation — 1.0 = A2, 1.2 = A3, 2.0 = A5 — so a table left Your PR breaks that chain in two places and needs both:
Each half is load-bearing. Worth stating in the description — right now the two read as unrelated. But there is a smaller fix, and I think it should reframe the PRI moved the host file aside and re-ran everything against unmodified gpustack-runtime
Then end-to-end through The file is user-maintained state: That does not make this PR pointless — it makes it defence rather than repair. It stops a The one change I'd ask you to drop: mounting the whole
|
Supersedes #25. yxf0314's diagnosis was right that multi-card A5 needed a change; this takes the part of it that holds up, and drops the rest. A5 loads libhccl_v2.so, which accepts a 2.0 ranktable only. Ranktable format is bound to the chip generation -- 1.0 for A2, 1.2 for A3, 2.0 for A5 -- so a table left over from an older fleet is refused, not ignored: HCCL fails with Config_Error_Ranktable(EI0014) and multi-card init never happens. Isolated on an 8x Ascend950PR host: mounting only /etc/hccl_rootinfo.json into an otherwise working container reproduces EI0014, mounting only driver/topo does not. Two things follow, and nothing else does. GPUStack stops mounting the file on A5. This is its own mount list only, so it covers the CDI path. Older generations keep the file, which is correct for them. Under the default Env policy the mount is ascend-docker-runtime's -- addUBMount is a bare os.Stat with no version check -- so GPUStack cannot prevent it and reporting is the only available action. A one-shot warning names EI0014 and says the file must be absent or 2.0. An absent file is the healthy host and stays silent; so does an older generation with its own table, since only the A5 row of the generation mapping has a measured failure behind it. This is defence, not repair: with the host file moved aside, unmodified gpustack-runtime already runs GLM-5.3-Flash TP=8 under the default policy. What it stops is a node's leftover ranktable silently killing multi-card workloads on it. Deliberately not included, having been measured and found unnecessary: A Device injection policy. It adds a configuration knob that does nothing by default, on the Env path where the vendor runtime does the mounting -- so it would not protect anyone who had not already been told to configure it. Its isolation is also nominal in the shape GPUStack deploys in, since a privileged container sees every device node whatever it was granted. Mounting the whole /usr/local/Ascend/driver for A5. driver/ube_mgmt is a staging directory for upgrade-tool holding one zero-byte lock file; no library under driver/lib64 references it, and the urma symbols are already inside the lib64 mount. The tree would also carry upgrade-tool and hccn_tool into a container holding /dev/davinci_manager, plus the CA store and 2570 kernel sources.
* fix(ascend): stop A5 from receiving a ranktable its HCCL rejects Supersedes #25. yxf0314's diagnosis was right that multi-card A5 needed a change; this takes the part of it that holds up, and drops the rest. A5 loads libhccl_v2.so, which accepts a 2.0 ranktable only. Ranktable format is bound to the chip generation -- 1.0 for A2, 1.2 for A3, 2.0 for A5 -- so a table left over from an older fleet is refused, not ignored: HCCL fails with Config_Error_Ranktable(EI0014) and multi-card init never happens. Isolated on an 8x Ascend950PR host: mounting only /etc/hccl_rootinfo.json into an otherwise working container reproduces EI0014, mounting only driver/topo does not. Two things follow, and nothing else does. GPUStack stops mounting the file on A5. This is its own mount list only, so it covers the CDI path. Older generations keep the file, which is correct for them. Under the default Env policy the mount is ascend-docker-runtime's -- addUBMount is a bare os.Stat with no version check -- so GPUStack cannot prevent it and reporting is the only available action. A one-shot warning names EI0014 and says the file must be absent or 2.0. An absent file is the healthy host and stays silent; so does an older generation with its own table, since only the A5 row of the generation mapping has a measured failure behind it. This is defence, not repair: with the host file moved aside, unmodified gpustack-runtime already runs GLM-5.3-Flash TP=8 under the default policy. What it stops is a node's leftover ranktable silently killing multi-card workloads on it. Deliberately not included, having been measured and found unnecessary: A Device injection policy. It adds a configuration knob that does nothing by default, on the Env path where the vendor runtime does the mounting -- so it would not protect anyone who had not already been told to configure it. Its isolation is also nominal in the shape GPUStack deploys in, since a privileged container sees every device node whatever it was granted. Mounting the whole /usr/local/Ascend/driver for A5. driver/ube_mgmt is a staging directory for upgrade-tool holding one zero-byte lock file; no library under driver/lib64 references it, and the urma symbols are already inside the lib64 mount. The tree would also carry upgrade-tool and hccn_tool into a container holding /dev/davinci_manager, plus the CA store and 2570 kernel sources. * fixup! fix(ascend): stop A5 from receiving a ranktable its HCCL rejects
2 participants
On Docker, the default Env injection policy sets the visible-devices env (ASCEND_VISIBLE_DEVICES), which makes the Ascend Docker Runtime apply device isolation. On A5 (950) that isolation hides the UB fabric, so HCCL rootInfo detection fails. The CDI mount profile, modeled on the A2/A3 operator profile, also omits A5's UB driver components (ube_mgmt, under the driver tree) and blanket-mounts a host /etc/hccl_rootinfo.json whose stale content fails rootInfo detection.